Support mobile display #822

pavloburchak · 2023-08-08T22:47:57Z

This PR...

Changes

Fixes

How to test it

screenshots

Checklist

tested locally
added new dependencies
updated the docs
added a test

vercel · 2023-08-08T22:48:02Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
testkube-dashboard	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 11, 2023 2:04pm

rangoo94

The most important thing is that we should:

avoid depending on JS for displaying on different devices (always when it's possible), and
avoid setting direct dimensions of elements (unless it's part of their geometry, or would be much harder otherwise)
This way, we may depend on browser mechanisms, ensuring that the behavior is smooth; mobile devices are worse than desktop, so it's even more important for them.

Besides code review, I've checked how it works in real, and works nicely 👍

Screenshot	Description
	Priority: `High` May be worth to delete help for creating executors on mobile, or make it smaller
	Priority: `Medium` Modal for creating the test should be constrained to maximum width
	Priority: `High` May be worth deleting the creating tests help on mobile
	Priority: `Low` It should wrap (or at least truncate?) for some minimum size of these items
	Priority: `Low` I'm thinking about this one - would it be possible to move the graph to right side of labels? If that would require some bigger effort - ignore it 🙂
	Priority: `Low` Should "Condition" / "Actions" steps list wrap?
	Priority: `High` The tabs are strangely positioned - the tab content should be scrollable, but tabs rather always visible
	Priority: `Very Low` Completely minor - maybe it's worth to change minimum size for official executors, to make them 2 in a row on mobile?

rangoo94 · 2023-08-09T06:47:00Z

src/components/molecules/EntityGrid/EntityGrid.styled.tsx

+  @media ${maxDevice.tablet} {
+    max-width: 75vw;
+  }
+
+  @media ${maxDevice.mobileL} {
+    max-width: 60vw;
+  }


ItemRow is a content of grid item, not a grid item itself, therefore it's width should be full grid item

The contents of ItemRow are px based, and the container is local, so it shouldn't be based on vw

The contents of ItemRow differ, so it shouldn't be generic

You may use minItemWidth prop of EntityGrid to define minimum size of item if it's needed

rangoo94 · 2023-08-09T06:48:18Z

src/components/molecules/EntityGrid/EntityGridItemPure.tsx

+  const {width} = useWindowSize();
+  const isDisplayExecutionTime = width > size.mobileL;


This should be done with CSS - will be more performant, and will always work; the JS here may not necessarily work nicely, especially given that it's pure component.

you mean with display none?

Yes, exactly 🙂

rangoo94 · 2023-08-09T06:51:54Z

src/components/organisms/EntityList/EntityListFilters/EntityListFilters.tsx

 import LabelsFilter from './LabelsFilter';
 import StatusFilter from './StatusFilter';
 import TextSearchFilter from './TextSearchFilter';

 const EntityListFilters: FC<FilterProps> = props => {
  const {isFiltersDisabled, ...rest} = props;

+  const isMobile = useIsMobile();
+
+  const filtersWidth = isMobile ? 'inherit' : '296px';


inherit is incorrect here, as you don't want to copy the property from parent. Most likely you've meant either initial or 100%.

I think I do want it, but ok, I can change it

initial would make the container use its default behavior, while inherit would "copy" the same value as the parent container - not the computed one, but the relative one.

As an example, if you'll have HTML (codesandbox):

<style> div {width: 30%;height: 20px;background: rgba(0, 0, 0, .1);} .inherit {width: inherit;} .initial {width: initial;} </style> <div><div class="inherit"><div class="inherit"></div></div></div> <br /> <div><div class="initial"><div class="initial"></div></div></div>

The first container will get smaller and smaller (as it will copy 30%), and the other one will take the parent's container size (as it will reset to defaults).

Therefore, I believe that you did rather want initial, but I may be wrong 🙂

Thanks, will update it

rangoo94 · 2023-08-09T06:54:18Z

src/components/organisms/EntityList/EntityListFilters/EntityDetailsFilters.styled.tsx

+  gap: 16px;
+  align-items: center;
+
+  ${props => (props.isMobile ? `width: 100%;` : '')}


Stretching items for all dimensions will work well instead (it may require adjusting the parent container too, but it may be worth it)

We should control CSS without JS, using media queries only

rangoo94 · 2023-08-09T06:58:01Z

src/components/molecules/FilterMenu/MenuFilter.styled.tsx

  position: relative;
  display: flex;
  justify-content: space-between;
  align-items: center;

-  width: 296px;
+  width: ${props => props.$width || '296px'};


Looking at the FiltersContainer code above, we should be able to easily get rid of width at all here

rangoo94 · 2023-08-09T06:58:47Z

src/components/organisms/EntityList/EntityListFilters/TextSearchFilter/TextSearchFilter.tsx

@@ -44,7 +44,7 @@ const TextSearchFilter: React.FC<FilterProps> = props => {
      value={inputValue}
      data-cy="search-filter"
      disabled={isFiltersDisabled}
-      style={{width: '296px'}}
+      style={{width}}


We may get rid of width, making it flexible instead

rangoo94 · 2023-08-09T07:02:33Z

src/components/organisms/Sider/Sider.styled.tsx

  position: fixed;
  display: flex;
  flex-direction: column;
  justify-content: space-between;

  padding-top: 30px;
-  width: 100px;
+  width: ${props => props.$width}px;


I would consider zoom instead, applied via media queries. I don't think there would be any edge cases, but what do you think?

.ant-layout-sider { zoom: 0.6; } .ant-layout-sider-children > div > div { zoom: 1.35; }

Haven't seen this approach later, this one is not looking perfectly in mobile

pavloburchak · 2023-08-09T21:19:19Z

for me these tabs display ok, could you provide more details?

rangoo94 · 2023-08-10T06:51:57Z

for me these tabs display ok, could you provide more details?

The tabs may not be visible, as the whole container (along with tabs) becomes scrollable on smaller heights (= mobile). The tabs should be rather always visible though, while only content (i.e. Log Output) should scroll.

rangoo94

Screenshot	Description
	Displaying the message - the buttons should wrap, and optionally we could make it smaller
	Full screen Log Output doesn't work
	Optional: it would be great if the full screen icon would go down. Also, probably the "AI Insights" tooltip could have fixed position, but I can do that in a separate PR.

src/components/organisms/EntityList/EntityListFilters/StatusFilter/StatusFilter.tsx

src/components/organisms/EntityList/EntityListFilters/TextSearchFilter/TextSearchFilter.tsx

src/components/organisms/Sider/Sider.tsx

src/styles/globalStyles.ts

* mobile support for filters, sider and list * entity details and settings optimization * remove unused * some pr fixes * hind adjustment * more mobile improvements * fix tabs * fix navigation for cloud * cloud message fix * fix: log output display / sider * fix: scrolling log output / positioning AI hints promo * simplify scrolling further * fix: full screen log output * fixup stylelint * chore: delete unused parts --------- Co-authored-by: Dawid Rusnak <[email protected]>

pavloburchak requested review from exu and Toxer10 as code owners August 8, 2023 22:47

pavloburchak requested review from rangoo94, mortada-codes and haneabogdan August 8, 2023 22:48

vercel bot deployed to Preview August 8, 2023 22:49 View deployment

rangoo94 reviewed Aug 9, 2023

View reviewed changes

vercel bot deployed to Preview August 9, 2023 21:21 View deployment

vercel bot deployed to Preview August 10, 2023 15:48 View deployment

vercel bot deployed to Preview August 10, 2023 21:16 View deployment

vercel bot deployed to Preview August 10, 2023 21:29 View deployment

pavloburchak added 8 commits August 11, 2023 01:03

mobile support for filters, sider and list

62f416e

entity details and settings optimization

78e42af

remove unused

a616351

some pr fixes

9a06005

hind adjustment

e63e041

more mobile improvements

5ac2fcb

fix tabs

e9e2aef

fix navigation for cloud

60b88be

pavloburchak force-pushed the pavlo/support-mobile-display branch from 04067ca to 60b88be Compare August 10, 2023 22:04

vercel bot deployed to Preview August 10, 2023 22:06 View deployment

rangoo94 reviewed Aug 11, 2023

View reviewed changes

cloud message fix

473ca99

vercel bot deployed to Preview August 11, 2023 10:14 View deployment

fix: log output display / sider

9da4cc8

vercel bot deployed to Preview August 11, 2023 11:39 View deployment

fix: scrolling log output / positioning AI hints promo

15f6d66

vercel bot deployed to Preview August 11, 2023 12:46 View deployment

simplify scrolling further

3bfef85

vercel bot deployed to Preview August 11, 2023 12:48 View deployment

fix: full screen log output

6db56fa

vercel bot deployed to Preview August 11, 2023 13:18 View deployment

fixup stylelint

c183de2

vercel bot deployed to Preview August 11, 2023 13:22 View deployment

rangoo94 approved these changes Aug 11, 2023

View reviewed changes

chore: delete unused parts

6e01fe1

vercel bot deployed to Preview August 11, 2023 14:04 View deployment

rangoo94 merged commit 44517da into develop Aug 11, 2023
7 checks passed

rangoo94 deleted the pavlo/support-mobile-display branch August 11, 2023 14:11

rangoo94 mentioned this pull request Aug 16, 2023

Release v1.14.0 #829

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support mobile display #822

Support mobile display #822

pavloburchak commented Aug 8, 2023

vercel bot commented Aug 8, 2023 •

edited

Loading

rangoo94 left a comment

rangoo94 Aug 9, 2023

rangoo94 Aug 9, 2023

pavloburchak Aug 9, 2023

rangoo94 Aug 9, 2023

rangoo94 Aug 9, 2023

pavloburchak Aug 9, 2023

rangoo94 Aug 9, 2023 •

edited

Loading

pavloburchak Aug 9, 2023

rangoo94 Aug 9, 2023

rangoo94 Aug 9, 2023

rangoo94 Aug 9, 2023

rangoo94 Aug 9, 2023

pavloburchak Aug 9, 2023

pavloburchak commented Aug 9, 2023

rangoo94 commented Aug 10, 2023

rangoo94 left a comment

		const {width} = useWindowSize();
		const isDisplayExecutionTime = width > size.mobileL;

Support mobile display #822

Support mobile display #822

Conversation

pavloburchak commented Aug 8, 2023

Changes

Fixes

How to test it

screenshots

Checklist

vercel bot commented Aug 8, 2023 • edited Loading

rangoo94 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rangoo94 Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavloburchak commented Aug 9, 2023

rangoo94 commented Aug 10, 2023

rangoo94 left a comment

Choose a reason for hiding this comment

vercel bot commented Aug 8, 2023 •

edited

Loading

rangoo94 Aug 9, 2023 •

edited

Loading